Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rebased version of #15629 - prevent setting assigned_to without setting assigned_type #15907

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

uberbrady
Copy link
Collaborator

I somehow really messed up that PR, so I had to rebase it here. Sorry for the mess.

This makes it so, at the model level, you cannot set assigned_to without setting assigned_type on an asset - or vice-versa.

And I added a bunch of tests to help show that.

I had to do a little bit of modification to some of our factories - which were doing exactly this, so they started to fail.

author Brady Wetherington <bwetherington@grokability.com> 1728320853 +0100
committer Brady Wetherington <bwetherington@grokability.com> 1733158021 +0000

Prevent setting assigned_to without setting assigned_type

Fixed tests to include assigned_type when setting assigned_to

Add new tests for assigned_to without assigned_type

Added tighter validation to assigned_to and assigned_type, new tests

Fixed wrong comment

Fixed tests to include assigned_type when setting assigned_to

Add new tests for assigned_to without assigned_type

Fixed wrong comment
@uberbrady uberbrady requested a review from snipe as a code owner December 2, 2024 16:57
Copy link

what-the-diff bot commented Dec 2, 2024

PR Summary

  • Standardized API Response in Asset Creation
    The AssetsController has been revised to normalize the way the API responds when creating assets. This provides consistency in the data structure returned, which is beneficial for further integration and interpretation of results.

  • Improved Data Preparation in Asset Storage
    The 'assigned_to' field is no longer combined during the preparation of StoreAssetRequest, adding clarity and reducing ambiguity during data processing.

  • Added and Enhanced Validation Rules in Asset Model
    The validation rules in the Asset model have been refined. The 'assigned_to' field is now set to be required along with 'assigned_type'. This means that when an asset is being created or updated, both fields must be provided, ensuring improved data integrity.

  • Better Log of Updates in AssetObserver
    AssetObserver has been updated so that changes are logged only when modifications are made to the 'assigned_to' attribute. This improves the efficiency of data logging.

  • Improved Test Scenarios through Updated Asset Factory Calls
    The asset factory now includes 'assigned_type' when creating test scenarios. This enhances the accuracy of the tests and validates scenarios where 'assigned_to' and 'assigned_type' are included.

  • Added Validation Tests in Asset Storage
    Additional validation tests have been added in StoreAssetTest to accommodate scenarios when 'assigned_to' and 'assigned_type' are omitted or invalid. This bolsters the robustness of our testing suite.

  • New Tests for Update Scenarios
    Introducing tests in UpdateAssetTest has enhanced the handling of updates that require both 'assigned_to' and 'assigned_type', ensuring these scenarios are reliably covered.

  • Added Test for Model Behavior Validation
    A test was included in AssetTest to validate the condition of 'assigned_type' without 'assign_to'. This verifies the correct behavior of the model under these conditions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant